Restore go-to-definition for triple-slash lib references - #64088
Restore go-to-definition for triple-slash lib references#64088Daniel Rosenwasser (DanielRosenwasser) with Copilot wants to merge 4 commits into
Conversation
Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
| if path, ok := p.libFilesByName[name]; ok { | ||
| return p.filesByPath[path] |
There was a problem hiding this comment.
I don't get it - if we have a source file, then we have a path right? As far as I understand, we don't need libFilesByName, right?
There was a problem hiding this comment.
You’re right. p.libFiles already maps each resolved source-file path to its LibFile, so the lookup now matches LibFile.Name there and uses that path to retrieve the source file. Removed libFilesByName in c7a702e.
| t.Parallel() | ||
| defer testutil.RecoverAndFail(t, "Panic on fourslash test") | ||
| const content = `// @Filename: /src/app.ts | ||
| /// <reference lib="/*start*/e/*middle*/s202/*end*/5" />` |
There was a problem hiding this comment.
| /// <reference lib="/*start*/e/*middle*/s202/*end*/5" />` | |
| /// <reference lib="/*start*/es/*middle*/2025/*end*/" />` |
There was a problem hiding this comment.
Updated the test markers to /*start*/es/*middle*/2025/*end*/ and refreshed its baseline in c7a702e.
Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
typescript-automation
Bot
added
the
For Uncommitted Bug
| } | ||
| if sourceFile, ok := p.filesByPath[tspath.Path(path)]; ok { | ||
| return sourceFile | ||
| for path, libFile := range p.libFiles { |
There was a problem hiding this comment.
I guess this was a bug - these were not actual paths.
There was a problem hiding this comment.
Pull request overview
Restores go-to-definition for triple-slash library references in the language service.
Changes:
- Resolves references before the SourceFile fallback.
- Locates libraries through canonical metadata.
- Adds fourslash coverage and baselines.
Show a summary per file
| File | Description |
|---|---|
tsc/internal/ls/definition.go |
Reorders reference resolution. |
tsc/internal/compiler/program.go |
Resolves canonical library paths. |
tsc/internal/fourslash/tests/goToDefinitionTypeReferenceDirective_test.go |
Adds library-reference coverage. |
tsc/testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionLibReferenceDirective.baseline.jsonc |
Adds the new expected baseline. |
tsc/testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionSourceUnit.baseline.jsonc |
Updates path-reference output. |
tsc/testdata/baselines/reference/fourslash/goToDefinition/goToDefinitionTypeReferenceDirective.baseline.jsonc |
Updates type-reference output. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Balanced
| originSelectionRange, _ := l.createLspRangeFromNode(node, file) | ||
| if reference != nil && reference.file != nil { | ||
| return l.createDefinitionLocations(originSelectionRange, clientSupportsLink, []*ast.Node{}, reference, spanmap.FeatureDefinition) | ||
| } | ||
| if node.Kind == ast.KindSourceFile { | ||
| return lsproto.LocationOrLocationsOrDefinitionLinksOrNull{} | ||
| } |
Jake Bailey (jakebailey)
left a comment
Jake Bailey (jakebailey)
left a comment
There was a problem hiding this comment.
I also found this bug separately while working on a fun Path refactor.
|
Copilot I actually think the origin selection range should be the text - I dunno, check what strada did |
Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
Verified: |
Description
Go-to-definition stopped resolving positions within
/// <reference lib="..." />directives in TypeScript 7.Changes
es2025reference./// <reference lib="es2025" />/// <reference lib="..." />directives #64086